fix(compile-sfc): handle mapped types work with omit and pick #12648
Conversation
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
edison1105
added
ready to merge
|
Caution Review failedThe pull request is closed. WalkthroughA new test was added to verify the correct resolution of mapped utility types combining Changes
Sequence Diagram(s)sequenceDiagram
participant TestCase
participant resolveStringType
participant resolveBuiltin
TestCase->>resolveStringType: resolve('OptionalTest')
resolveStringType->>resolveStringType: (recursive calls with typeParameters)
resolveStringType->>resolveBuiltin: (for Omit, Pick, Partial)
resolveBuiltin->>resolveStringType: resolve type arguments (with typeParameters)
resolveStringType-->>TestCase: resolved prop types
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
/ecosystem-ci run |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/compiler-sfc/src/script/resolveType.ts (1)
569-577: Generic parameter check is solid – minor nitpick on variable shadowing
const name = node.typeName.nameshadows the outer-scopenameidentifier introduced a few lines later for the built-in switch. Consider inlining the identifier (node.typeName.name) or choosing a more specific local (e.g.typeNameId) to avoid subtle shadowing mistakes during future refactors.🧰 Tools
🪛 Biome (1.9.4)
[error] 570-570: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts(1 hunks)packages/compiler-sfc/src/script/resolveType.ts(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/compiler-sfc/src/script/resolveType.ts
[error] 570-570: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: test / unit-test-windows
🔇 Additional comments (2)
packages/compiler-sfc/src/script/resolveType.ts (1)
686-700: 👍 Correctly forwards generics forPick/OmitPassing
typeParameterswhen resolving the second argument fixes the original issue and is implemented consistently for both utility types.🧰 Tools
🪛 Biome (1.9.4)
[error] 695-700: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts (1)
281-296: Great regression testThe new case reproduces the reported failure and protects against future regressions. Test reads clearly and asserts both props – thanks!
|
📝 Ran ecosystem CI: Open
|
close #12647
Summary by CodeRabbit